-
Notifications
You must be signed in to change notification settings - Fork 537
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(LabelGroup): render as list by default #5156
feat(LabelGroup): render as list by default #5156
Conversation
🦋 Changeset detectedLatest commit: c9675ef The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
size-limit report 📦
|
…5-prclabelgroup-component-lacks-semantic-structure
…acks-semantic-structure
🟢 golden-jobs completed with status |
…acks-semantic-structure
👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/348056 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
<StyledLabelGroupContainer data-overflow="inline" data-list={isList || undefined} sx={sxProp} as={as}> | ||
{isList | ||
? React.Children.map(children, child => { | ||
return <li>{child}</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these need a key
prop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! done , Thanks!!
…5-prclabelgroup-component-lacks-semantic-structure
Closes https://github.com/github/primer/issues/3425
Renders the
LabelGroup
as aul
withli
s by default, with the option to useas
prop to customize the element type of the rendered container. In the case other thanul
orol
, the children will be rendered asspan
.Changelog
New
as
prop inLabelGroup
to customize container element (provides backwards compatibility)ul
inLabelGroup
data-list
attribute inStyledLabelGroupContainer
(used to apply styles conditionally)LabelGroup
children now wrapped by<li></li>
component when rendered as listLabelGroup
tests to account for new functionality and use casesChanged
LabelGroup
renders as aul
by default, with option to override default implementation viaas
prop.Rollout strategy
Testing & Reviewing
Check DOM in deployed story
, verify it renders as a
ul
withli
s. No visual regressions should be present.Merge checklist